-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix][broker] Flaky-test: TopicTransactionBufferTest.testMessagePublishInOrder #24826
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Technoboy-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also delete the useless field publishFuture, transactionBufferFuture defined in the TransactionBufferTestImpl
…shInOrder (apache#24788) Signed-off-by: Dream95 <[email protected]>
78c24a0 to
f571cc6
Compare
...r/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #24826 +/- ##
============================================
+ Coverage 74.28% 74.30% +0.01%
- Complexity 33842 33849 +7
============================================
Files 1913 1913
Lines 149315 149317 +2
Branches 17331 17331
============================================
+ Hits 110915 110946 +31
+ Misses 29558 29520 -38
- Partials 8842 8851 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…shInOrder (#24826) Signed-off-by: Dream95 <[email protected]> Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit 8828734)
…shInOrder (#24826) Signed-off-by: Dream95 <[email protected]> Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit 8828734)
…shInOrder (apache#24826) Signed-off-by: Dream95 <[email protected]> Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit 8828734) (cherry picked from commit 74c366c)
…shInOrder (apache#24826) Signed-off-by: Dream95 <[email protected]> Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit 8828734) (cherry picked from commit 74c366c)
Fixes #24788
Fixes #24509
Motivation
The test is flaky.
Reproduce the issue:
Add Thread.sleep after sendAsync to avoid batching sends.
The issue occurs because the in this test, the TransactionBufferTestImpl class extends TopicTransactionBuffer and declares a field with the same name publishFuture.
Therefore, in the appendBufferToTxn method, getPublishFuture() retrieves the publishFuture from the subclass, while the assignment
publishFuture = futuremodifies the publishFuture belonging to the parent class.Modifications
Use ‘setPublishFuture(future)’ to replace ‘publishFuture = future’
Verifying this change
This change is already covered by existing tests.
Does this pull request potentially affect one of the following parts:
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: Dream95#1